-
Notifications
You must be signed in to change notification settings - Fork 4.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update custom webview documentation with kotlin code examples #3060
Conversation
✅ Deploy Preview for react-native ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great stuff! 👍
Have you actually tried the code? As there are some odd fragments which I'd like to make sure they're tested before they end on the docs.
docs/custom-webview-android.md
Outdated
val name = "RCTCustomWebView" | ||
get() = Companion.field |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make this a const val
docs/custom-webview-android.md
Outdated
protected class CustomWebViewClient : ReactWebViewClient() | ||
protected class CustomWebView(reactContext: ThemedReactContext?) : ReactWebView(reactContext) | ||
|
||
protected fun createReactWebViewInstance(reactContext: ThemedReactContext?): ReactWebView { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fact that here the is no override
looks odd to me. Have you tested this code?
docs/custom-webview-android.md
Outdated
```kotlin | ||
class CustomWebViewManager : ReactWebViewManager() { | ||
protected class CustomWebView(reactContext: ThemedReactContext?) : ReactWebView(reactContext) { | ||
@Nullable var finalUrl: String? = null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Nullable
should not be used in Kotlin (that's part of the type system).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this can maybe be an inner
class
docs/custom-webview-android.md
Outdated
```kotlin | ||
// NavigationCompletedEvent.kt | ||
class NavigationCompletedEvent(viewTag: Int, params: WritableMap) : | ||
Event<NavigationCompletedEvent?>(viewTag) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Event<NavigationCompletedEvent?>(viewTag) { | |
Event<NavigationCompletedEvent>(viewTag) { |
docs/custom-webview-android.md
Outdated
// NavigationCompletedEvent.kt | ||
class NavigationCompletedEvent(viewTag: Int, params: WritableMap) : | ||
Event<NavigationCompletedEvent?>(viewTag) { | ||
private val mParams: WritableMap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line can go
docs/custom-webview-android.md
Outdated
fun shouldOverrideUrlLoading(view: WebView, url: String?): Boolean { | ||
val shouldOverride: Boolean = super.shouldOverrideUrlLoading(view, url) | ||
val finalUrl: String = (view as CustomWebView).getFinalUrl() | ||
if (!shouldOverride && url != null && finalUrl != null && String(url) == finalUrl) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (!shouldOverride && url != null && finalUrl != null && String(url) == finalUrl) { | |
if (!shouldOverride && url != null && finalUrl != null && url == finalUrl) { |
docs/custom-webview-android.md
Outdated
|
||
```kotlin | ||
class CustomWebViewManager : ReactWebViewManager() { | ||
@get:Nullable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nullable should go
docs/custom-webview-android.md
Outdated
```kotlin | ||
class CustomWebViewManager : ReactWebViewManager() { | ||
@get:Nullable | ||
val exportedCustomDirectEventTypeConstants: Map |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
val exportedCustomDirectEventTypeConstants: Map | |
val exportedCustomDirectEventTypeConstants: Map<String, Any?> |
docs/custom-webview-android.md
Outdated
var export: Map<String?, Any?> = super.getExportedCustomDirectEventTypeConstants() | ||
if (export == null) { | ||
export = MapBuilder.newHashMap() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var export: Map<String?, Any?> = super.getExportedCustomDirectEventTypeConstants() | |
if (export == null) { | |
export = MapBuilder.newHashMap() | |
} | |
val export = if (super.getExportedCustomDirectEventTypeConstants() != null) { | |
super.getExportedCustomDirectEventTypeConstants() | |
} else { | |
export = MapBuilder.newHashMap() | |
} |
docs/custom-webview-android.md
Outdated
export.put( | ||
"navigationCompleted", | ||
MapBuilder.of("registrationName", "onNavigationCompleted") | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export.put( | |
"navigationCompleted", | |
MapBuilder.of("registrationName", "onNavigationCompleted") | |
) | |
export["navigationCompleted"] = MapBuilder.of("registrationName", "onNavigationCompleted") |
Not sure if you have set
access here, but is worth to try.
I have tested some of them, but you're right. I really should've tested everything. Don't worry I will test properly and update the requested changes, as soon as I can. You can take a look again then. |
Great thanks for doing that. Ping one of us again once you're done with it 👍 |
Hey @cortinico I was thinking maybe I can use 'RNCWebview' by react native community in examples instead of ReactWebView. As the docs already recommend that, which is why I had problems testing the code in the first place because references were not resolving in the latest react native version. ReactWebViewManager -> RNCWebViewManager If yes, I can maybe update the java code in a separate PR? |
Totally. I'd say let's hold this PR for now, update the Java code to use react-native-webview and get back to this once the first one is merged. |
Sure, awesome. I will make a separate pr for java. |
588b1f2
to
2a942be
Compare
@cortinico I have updated the code. Have a look when you can. |
docs/custom-webview-android.md
Outdated
|
||
companion object { | ||
/* This name must match what we're referring to in JS */ | ||
const val name = "RCTCustomWebView" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const val name = "RCTCustomWebView" | |
const val REACT_CLASS = "RCTCustomWebView" |
docs/custom-webview-android.md
Outdated
|
||
```kotlin | ||
class CustomWebViewManager : RNCWebViewManager() { | ||
protected inner class CustomWebView(reactContext: ThemedReactContext?) : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
protected inner class CustomWebView(reactContext: ThemedReactContext?) : | |
protected inner class CustomWebView(reactContext: ThemedReactContext?, var finalUrl: String? = null) : RNCWebView(reactContext) |
docs/custom-webview-android.md
Outdated
class NavigationCompletedEvent(viewTag: Int, val params: WritableMap) : | ||
Event<NavigationCompletedEvent>(viewTag) { | ||
private val eventName: String = "navigationCompleted" | ||
override fun getEventName() = this.eventName |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
override fun getEventName() = this.eventName | |
override fun getEventName() : String = "navigationCompleted" |
docs/custom-webview-android.md
Outdated
// NavigationCompletedEvent.kt | ||
class NavigationCompletedEvent(viewTag: Int, val params: WritableMap) : | ||
Event<NavigationCompletedEvent>(viewTag) { | ||
private val eventName: String = "navigationCompleted" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove this
docs/custom-webview-android.md
Outdated
class NavigationCompletedEvent(viewTag: Int, val params: WritableMap) : | ||
Event<NavigationCompletedEvent>(viewTag) { | ||
|
||
private val eventName = "navigationCompleted" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
docs/custom-webview-android.md
Outdated
val export = | ||
if (super.getExportedCustomDirectEventTypeConstants() != null) { | ||
super.getExportedCustomDirectEventTypeConstants() | ||
} else { | ||
MapBuilder.newHashMap<Any, Any?>() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
val export = | |
if (super.getExportedCustomDirectEventTypeConstants() != null) { | |
super.getExportedCustomDirectEventTypeConstants() | |
} else { | |
MapBuilder.newHashMap<Any, Any?>() | |
} | |
val super = super.getExportedCustomDirectEventTypeConstants() | |
val export = if (super != null) { | |
super | |
} else { | |
MapBuilder.newHashMap<Any, Any?>() | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super is a keyword, it cannot be used as a variable name, should I change to something else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah my bad 🤦♂️ superTypeConstants
also work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
docs/custom-webview-android.md
Outdated
export?.set( | ||
"navigationCompleted", | ||
MapBuilder.of("registrationName", "onNavigationCompleted") | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export?.set( | |
"navigationCompleted", | |
MapBuilder.of("registrationName", "onNavigationCompleted") | |
) | |
export["navigationCompleted"] = | |
MapBuilder.of("registrationName", "onNavigationCompleted") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It shows error "only safe or non-null asserted calls are allowed on receiver"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should work once you refactor the rest of the code as export
will be non nullable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, done
@cortinico, I have completed the changes. Check out when you can. |
Yup. Looks good but there are a couple of minor things to address still 👍 |
val superTypeConstants = super.getExportedCustomDirectEventTypeConstants() | ||
val export = superTypeConstants ?: MapBuilder.newHashMap<Any, Any?>() | ||
export["navigationCompleted"] = MapBuilder.of("registrationName", "onNavigationCompleted") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
This PR adds kotlin code examples in custom webview documentation.
Related to #3018